[clang-tidy][NFC] Enable 'performance-move-const-arg' in '.clang-tidy' config #148549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vbvictor
requested review from
PiotrZSL,
carlosgalvezp,
5chmidti and
HerrCai0907
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesSet I find std::move here useful, WDYT?
With option set to diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
index a179d4bf66b4..5217dd4a50b6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -502,7 +502,7 @@ struct Mix {
MixData Data;
Mix(const ParmVarDecl *F, const ParmVarDecl *S, MixData Data)
- : First(F), Second(S), Data(std::move(Data)) {}
+ : First(F), Second(S), Data(Data) {}
void sanitize() { Data.sanitize(); }
MixFlags flags() const { return Data.Flags; }
@@ -1476,7 +1476,7 @@ static MixableParameterRange modelMixingRange(
assert(M.flagsValid() && "All flags decayed!");
if (M.mixable())
- MixesOfIth.emplace_back(std::move(M));
+ MixesOfIth.emplace_back(M);
}
if (MixesOfIth.empty()) {
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
index 0b6b8d9c9713..5c2d97b20518 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -132,7 +132,7 @@ void SpecialMemberFunctionsCheck::check(
llvm::SmallVectorImpl<SpecialMemberFunctionData> &Members =
ClassWithSpecialMembers[ID];
if (!llvm::is_contained(Members, Data))
- Members.push_back(std::move(Data));
+ Members.push_back(Data);
};
if (const auto *Dtor = Result.Nodes.getNodeAs<CXXMethodDecl>("dtor")) {
diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
index e9b96c4016af..a506e01a637b 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
@@ -145,7 +145,7 @@ matchEnableIfSpecialization(TypeLoc TheType) {
TheType = Qualified.getUnqualifiedLoc();
if (auto EnableIf = matchEnableIfSpecializationImpl(TheType))
- return EnableIfData{std::move(*EnableIf), TheType};
+ return EnableIfData{*EnableIf, TheType};
return std::nullopt;
}
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index 2af67f7ccb4c..7b1c2b4cb839 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -38,7 +38,7 @@ namespace {
struct NotLengthExprForStringNode {
NotLengthExprForStringNode(std::string ID, DynTypedNode Node,
ASTContext *Context)
- : ID(std::move(ID)), Node(std::move(Node)), Context(Context) {}
+ : ID(std::move(ID)), Node(Node), Context(Context) {}
bool operator()(const internal::BoundNodesMap &Nodes) const {
// Match a string literal and an integer size or strlen() call.
if (const auto *StringLiteralNode = Nodes.getNodeAs<StringLiteral>(ID)) {
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index fab2365f1147..6f94d991a4e1 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -280,7 +280,7 @@ IdentifierNamingCheck::FileStyle IdentifierNamingCheck::getFileStyleFromOptions(
Options.get<IdentifierNamingCheck::CaseType>(StyleString);
if (CaseOptional || Prefix || Postfix || IgnoredRegexpStr || HPTOpt)
- Styles[I].emplace(std::move(CaseOptional), Prefix.value_or(""),
+ Styles[I].emplace(CaseOptional, Prefix.value_or(""),
Postfix.value_or(""), IgnoredRegexpStr.value_or(""),
HPTOpt.value_or(IdentifierNamingCheck::HPT_Off));
}
Full diff: https://github.com/llvm/llvm-project/pull/148549.diff 5 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/.clang-tidy b/clang-tools-extra/clang-tidy/.clang-tidy
index 2443c979621da..be8e598de9634 100644
--- a/clang-tools-extra/clang-tidy/.clang-tidy
+++ b/clang-tools-extra/clang-tidy/.clang-tidy
@@ -16,7 +16,6 @@ Checks: >
-modernize-use-trailing-return-type,
performance-*,
-performance-enum-size,
- -performance-move-const-arg,
-performance-no-int-to-ptr,
-performance-type-promotion-in-math-fn,
-performance-unnecessary-value-param,
@@ -39,3 +38,7 @@ Checks: >
-readability-static-definition-in-anonymous-namespace,
-readability-suspicious-call-argument,
-readability-use-anyofallof
+
+CheckOptions:
+ - key: performance-move-const-arg.CheckTriviallyCopyableMove
+ value: false
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index 82fd3316b942a..82d1cf13440bc 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -132,8 +132,7 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
hasAncestor(functionDecl().bind("func")),
hasAncestor(functionDecl(
isDefinition(), equalsBoundNode("func"), ToParam,
- unless(anyOf(isDeleted(),
- hasDescendant(std::move(ForwardCallMatcher))))))),
+ unless(anyOf(isDeleted(), hasDescendant(ForwardCallMatcher)))))),
this);
}
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
index 0804aa76d953c..ee70911dff1d5 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
@@ -67,7 +67,7 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) {
hasParent(fieldDecl(
hasParent(recordDecl(isExternCContext())))),
hasAncestor(functionDecl(isExternC())))),
- std::move(IgnoreStringArrayIfNeededMatcher))
+ IgnoreStringArrayIfNeededMatcher)
.bind("typeloc"),
this);
}
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdNumbersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdNumbersCheck.cpp
index ff5a302dfa75d..934cc24817d73 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStdNumbersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdNumbersCheck.cpp
@@ -319,7 +319,7 @@ void UseStdNumbersCheck::registerMatchers(MatchFinder *const Finder) {
Finder->addMatcher(
expr(
- anyOfExhaustive(std::move(ConstantMatchers)),
+ anyOfExhaustive(ConstantMatchers),
unless(hasParent(explicitCastExpr(hasDestinationType(isFloating())))),
hasType(qualType(hasCanonicalTypeUnqualified(
anyOf(qualType(asString("float")).bind("float"),
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index e421c9f11b24b..25601f9a01a48 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -149,7 +149,7 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) {
}
Finder->addMatcher(
callExpr(
- callee(functionDecl(hasAnyName(std::move(Names)))
+ callee(functionDecl(hasAnyName(Names))
.bind((FuncDecl + Twine(Replacers.size() - 1).str()))),
ast_matchers::internal::DynTypedMatcher::constructVariadic(
ast_matchers::internal::DynTypedMatcher::VO_AnyOf,
|
|
Are they really need to "std::move" in the patch? |
Do you mean with |
Adding that keeping I have a small preference for |
|
Merging it as is with |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/20989 Here is the relevant piece of the build log for the reference |
krishna2803
pushed a commit
to krishna2803/llvm-project
that referenced
this pull request
Aug 12, 2025
…' config (llvm#148549) Set `performance-move-const-arg.CheckTriviallyCopyableMove` option to `false` because "trivially copyable" is too strict and give warning for e.g. `MixData` class: https://github.com/llvm/llvm-project/blob/1fbfa333f64bf714efa84db6b1075fc864d53bf8/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp#L389 Here: https://github.com/llvm/llvm-project/blob/1fbfa333f64bf714efa84db6b1075fc864d53bf8/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp#L504-L505 I find `std::move` here useful.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Set
performance-move-const-arg.CheckTriviallyCopyableMoveoption tofalsebecause "trivially copyable" is too strict and give warning for e.g.MixDataclass:llvm-project/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
Line 389 in 1fbfa33
llvm-project/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
Lines 504 to 505 in 1fbfa33
I find
std::movehere useful, WDYT?With option set to
true(default) we get these more warnings which are only about trivially copyable types: